Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ci: add action for python linting and formatting #12

Merged
merged 4 commits into from
Jun 5, 2024
Merged

ci: add action for python linting and formatting #12

merged 4 commits into from
Jun 5, 2024

Conversation

nathan-weinberg
Copy link
Member

Implemented this locally in #7 but missed this action

@nathan-weinberg nathan-weinberg force-pushed the lint-action branch 2 times, most recently from 99f2ae4 to eb9ca79 Compare June 4, 2024 19:35
@nathan-weinberg
Copy link
Member Author

Only CI failure here is due to #6

russellb
russellb reviewed Jun 4, 2024
View reviewed changes
requirements.txt Outdated Show resolved Hide resolved
@bjhargrave
Copy link
Contributor

We should also add the mypy step from instructlab/instructlab#1235 once it is completed.

@tiran
Copy link
Contributor

tiran commented Jun 5, 2024

You also need to add a py.typed marker file to src/instructlab/sdg, so the parent package knows that the subpackage uses type annotations. https://peps.python.org/pep-0561/#packaging-type-information

@nathan-weinberg
Copy link
Member Author

We should also add the mypy step from instructlab/instructlab#1235 once it is completed.

As part of this PR or as a followup?

You also need to add a py.typed marker file to src/instructlab/sdg, so the parent package knows that the subpackage uses type annotations. https://peps.python.org/pep-0561/#packaging-type-information

That path doesn't exist - do you mean src/instructlab_sdg? What should this file look like?

@nathan-weinberg
Copy link
Member Author

@bjhargrave since the MyPy PR merged in the other repo I went ahead and added it to this PR

@nathan-weinberg
Updated Makefile to have make verify be single command for all Pyth…
e9a2898 
…on checks

This is a slight reorg I just think makes more sense - anyone touching Python
code can run `make verify` as a simple sanity check for their code. Spellcheck
is more Markdown-specific and already has its own Makefile command so replaced
'spellcheck' with the new 'mypy` tox env

Signed-off-by: Nathan Weinberg <[email protected]>
@nathan-weinberg nathan-weinberg requested a review from russellb June 5, 2024 14:51
@russellb russellb merged commit 7fce978 into instructlab:main Jun 5, 2024
2 checks passed
@nathan-weinberg nathan-weinberg deleted the lint-action branch June 5, 2024 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@russellb russellb russellb approved these changes

@bjhargrave bjhargrave bjhargrave approved these changes

@tiran tiran Awaiting requested review from tiran

@aakankshaduggal aakankshaduggal Awaiting requested review from aakankshaduggal

@oindrillac oindrillac Awaiting requested review from oindrillac

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nathan-weinberg @bjhargrave @tiran @russellb